fix(app-shell): carry default and visibleWhen through the object-field options editor - #7629

Fixes #7540

Shape 1 only, as scoped by the dispatching PM on the card: the reader keeps the
whole authored option and the writer puts back what it did not display. No
per-option authoring UI was added — see "Deliberately not done".

What changed

One source file: packages/app-shell/src/views/metadata-admin/inspectors/ObjectFieldInspector.tsx.

The loss started in the reader, which is why a writer-only patch could not
have closed it. readOptions projected every authored option down to exactly
value / label / color, so default and visibleWhen were already gone
before patchOptions ever ran — whatever the writer were taught to carry, it
can only carry what the reader handed it.

All three parts move together, as the card's fence requires:

• interface Option (module-local) gains a rest slot for the authored
  keys the editor has no control for.
• readOptions now keeps the whole authored option, parking everything
  outside the three displayed keys on rest. The slot is only attached when
  there is something to carry, so an option with nothing extra produces a row
  byte-identical to what this reader used to produce.
• patchOptions spreads that carrier's contents back into the written
  document, displayed keys last so they stay authoritative for their own slots.

rest is editor-internal and never becomes a document key: SelectOptionSchema
is strict, so leaking it would 422 every save. The writer therefore emits a
plain document object and never spreads an Option. A test pins the exact key
set for this.

This mirrors readFields in previews/object-fields-io.ts one level up, which
preserves unknown keys on a field definition and strips only the named retired
keys a shipped build actually wrote (RETIRED_FIELD_KEYS). No counterpart
tombstone list for option keys is owed: all five entries in the registry
(indexed, referenceTo, formula, isSystem, sortOrder) are field-level,
and this editor has only ever written the three keys it displays.

The round trip, measured

Not asserted — these are the observed documents from the ablation run below,
same input, same edit ("edit the Label box"), reader reverted vs. repaired.

Input option: { value: 'alpha', label: 'Alpha', color: '#00ff00', default: true, visibleWhen: 'record.tier == 2' }

And the key set directly, from the carrier-leak pin:

Re-measured on the installed @objectstack/spec 17.2.0, controls lit before any
row was trusted (the card's table, reproduced independently here):

CONTROL clean  { value:'alpha', label:'A' }        ACCEPT  parsed={"label":"A","value":"alpha"}
NEG-CONTROL    { value:'a',     label:'A' }        REJECT  too_small@[value]
default: true                                      ACCEPT  parsed={…,"default":true}
visibleWhen: "x > 1"   (string)                    ACCEPT  parsed={…,"visibleWhen":{"dialect":"cel","source":"x > 1"}}
visibleWhen: {dialect:'cel', source:'x > 1'}       ACCEPT  same envelope
visibleWhen: true      (boolean)                   REJECT  invalid_union@[visibleWhen]
CONTROL unknown key { …, zzz:1 }                   REJECT  unrecognized_keys@[](zzz)
label omitted                                      REJECT  invalid_type@[label]
label: ''                                          ACCEPT  (the #7014 Q2 repair, untouched)

Two traps inherited from the card rather than re-earned: a select option's
value must be at least 2 characters (a 1-character value poisons every row
with too_small@[value] and the visibleWhen rows then say nothing), and the
boolean form of visibleWhen is refused — it is the string form that is
accepted and canonicalized into the expression envelope.

Ablation

Restored the original readOptions projection only, leaving the repaired
writer in place — the sharpest test of the card's central claim that the reader
is the loss site. Predictions written before the run; observed matched
case-for-case, 10 of 10.

Mutation proven on disk before the run: old-projection anchor count 1 (expected
1), carrier-attach anchor count 0 (expected 0), blob
31c5cf3e to b490fcf0. No rebuild leg applies — the test imports
./ObjectFieldInspector, the relative source, not a package dist, so vitest
resolves the mutated file directly. Restore proven: blob back to 31c5cf3e
(equal to HEAD's) and git diff HEAD empty.

Totals: Tests 7 failed | 8 passed (15) ablated, 15 passed (15) repaired.

The third row is the one worth reading twice. Dropping both keys still yields
a document the contract ACCEPTS — which is exactly why this bug was silent
rather than a 422, and why an "is the payload valid?" test can never detect it.
That case is deliberately kept in the suite as a non-detector.

Verification

Every run below is on 7001725f, the tree as pushed, with a clean working tree.
Exit codes captured by redirecting to a file, never through a pipe.

Notes on how to read those:

• The 12 eslint warnings are pre-existing and unchanged, not introduced
  here: : any occurrences in this file are 1 at origin/main and 1 at HEAD
  (the raw.map((o: any) …) the original reader already had). 0 errors.
• The new test file really is type-checked. The package runs
  tsc --noEmit && tsc -p tsconfig.test.json; --listFiles on that second
  program contains both the new test file and the changed source (count 1 each,
  measured — not inferred from the task going green).

Declared narrowing: the full packages/app-shell/ suite

The suite is 615 test files; the 226-file metadata-admin subset alone took
8m34s on the shared box, so the whole package cannot finish inside this seat's
10-minute foreground ceiling. CI runs it in full regardless. What was run
instead is the complete transitive dependent closure, with the three things
that make a narrowing a measurement rather than a gap:

1. Population from the tool, not from me. vitest related walked its own
   module graph from the changed file and collected the dependents — 53 test
   files spanning exactly two directories, metadata-admin and
   studio-design. That is vitest's answer to "what can observe this file".
2. Counts from the tool's output. 53 files / 414 tests for the closure;
   226 files / 2341 tests for the whole containing subtree, run independently
   and green.
3. Invariance for everything untouched. The diff changes one source file.
   Its three touched symbols — interface Option, readOptions,
   patchOptions — are all module-local (no export; lines 95, 125, 449). The
   file's only export is the ObjectFieldInspector component at line 258, whose
   signature is unchanged, and the whole-repo type-check (81/81) confirms no
   type consumer moved. So no test outside that closure can observe this change.

Repo-wide pnpm lint is likewise CI-owned. Its relevance here is bounded by the
same invariance: eslint.config.js declares no projectService and no
parserOptions.project (type-aware linting is off), so a one-file diff cannot
move the verdict on any file it does not contain.

Housekeeping

Clause-②: no. Re-judged from this diff rather than inherited:

• Nothing published moves. The only exported symbol in the changed file is
  the ObjectFieldInspector component, and its props type
  (MetadataInspectorProps) is untouched. interface Option carries no
  export modifier — verified on this diff, at line 95 — so the widened type is
  not reachable from any other module, let alone from a published entry point.
• The accept set does not move. @objectstack/spec is not touched, and both
  keys were already ACCEPTED by SelectOptionSchema 17.2.0 before this PR
  (probe table above). The repair stops the editor dropping keys the
  contract already takes; it does not ask the contract to take anything new.

One behaviour change a reviewer should see plainly rather than discover, since
it is the honest cost of "preserve what the editor does not display": an option
carrying a key the spec refuses is now carried through instead of being
silently dropped, so a save of such a document surfaces
unrecognized_keys@[options[0]] where it previously succeeded by quietly
rewriting the author's document. I judge this correct and in-contract — the
document was already illegal before the inspector opened it (the pin asserts
exactly that, so the case cannot be misread as this PR creating a 422), the
editor declines to hide a pre-existing violation rather than laundering it, and
it is the same contract readFields keeps one level up. It is a behaviour
change, not a contract change: the published accept set is identical. Flagging
it because it is the one place where reasonable people could want the opposite
default; say so and I will switch to an explicit two-key allowlist.

Deliberately not done

• Shape 2 — per-option authoring UI (a default radio across the option
  list, a CelPredicateField per option). Ruled out of scope by the dispatching
  PM before work started: it is a genuine product design question about that
  panel. Shape 1 is a strict subset of it and forecloses nothing — the reader is
  the loss site, so shape 2 could only ever be reached through this change.
  There is now no per-option control for either key; both simply survive.
• No fenced path was touched. packages/components/src/ui/** (finding(components): packages/components/src/ui/chart.tsx is an unreferenced duplicate of the plugin-charts chart primitives, carrying the legend bug fixed in objectui#7248 #7397),
  packages/plugin-list/** and packages/core/src/utils/normalize-list-view.ts
  (ListView: fold data={{ provider: 'object', object }} → objectName and read specType/type for the view kind — step 6 of #2890, released by objectstack#14791 ruling B #7477), DashboardView.tsx (fix(dashboard): retire the dashboard-root title read on all five surfaces #7622) and content/docs/releases/ are all
  untouched — the diff is 3 files, listed above.
• The patchOptions label repair landed in fix(app-shell): an emptied picklist option Label stays a legal document (Q2) #7536 is left exactly as it is.
  Different keys, different mechanism. Its 5 pins run green in every run above,
  ablated and repaired.
• NOT MEASURED: no browser dogfood run. The change is verified at the
  component's write boundary (what onPatch persists) and against the contract,
  not by driving the designer in a real browser.
• NOT MEASURED: CI gate outcomes. Reported at draft-PR time per the dispatch
  contract; the gate results above are local runs only.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KbJQ1y1J12nZxYzFWhP8Q3

Generated by Claude Code